Skip to content

Третья домашняя работа - #3

Open
olerom wants to merge 9 commits into
masterfrom
dz_3
Open

Третья домашняя работа#3
olerom wants to merge 9 commits into
masterfrom
dz_3

Conversation

@olerom

@olerom olerom commented Jun 6, 2017

Copy link
Copy Markdown
Collaborator

Добавил основной функционал. Без модулей.

@olerom
olerom requested a review from DmitryMakhnev June 6, 2017 20:58

@DmitryMakhnev DmitryMakhnev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public/dist/scripts/dummy.js надо разбивать на компоненты, без разбития смотреть не буду

@DmitryMakhnev DmitryMakhnev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Первая и самая важная проблема в связке и выделении компонентов.
На лекциях мы обсуждали, что компоненты должны быть связаны событиями или работать от данных.
Разберите материалы лекций и мою реализацию дз в части связки компонентов

Comment thread public/dist/scripts/Application.js Outdated
var counter = new Counter(itemsLeft);
var addTodo = new AddTodo(input, counter, list);

var showAll = document.getElementsByClassName('filter')[0];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ищем внутренности компонента перед тем как его заинитить

Comment thread public/dist/scripts/Application.js Outdated
var active = document.getElementsByClassName('filter')[1];
var clearCompleted = document.querySelector('.js-todos-actions-bar_clear-completed');

var todoActionBar = new TodoActionBar(completed, active, clearCompleted, counter, showAll);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А что если фильтр добавлю?

Comment thread public/dist/scripts/Application.js Outdated

var input = document.querySelector('.todo-add_input');
var list = document.querySelector('.todo-list');
var itemsLeft = document.getElementsByClassName('js-todos-actions-bar_counter')[0];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему разные методы поиска элементов DOM?


list.appendChild(templateResult.root);

templateResult.deleteLink.addEventListener(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Метод компонента добавления развешивает события на внутреннсти компонента айтема списка.
Нарушается инкапсуляция компонента списка.

@@ -0,0 +1,100 @@
var READY_MARK = "__ready";

function AddTodo(input, counter, list) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Компонент добавления дирижирует списком.
Если надо будет подружить компонент с другим списком, надо будет всё переписывать (

}
};

this.showAll.addEventListener('click', function (e) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

На каждый фильт описана логика работы в лоб. Если произойдёт изменение количества фильтров, будет очень грустно(

this.showAll.addEventListener('click', function (e) {
e.preventDefault();

showAll.classList.add(ACTIVE_MARK);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кейс с тремя фильтрами повторяется в каждом фильтре. Неужели не кажется не правильным писать руками одно и тоже более одного раза?

showAll.classList.remove(ACTIVE_MARK);
active.classList.remove(ACTIVE_MARK);

var hidden = document.getElementsByClassName('todo-item');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В каждом фильтре берём и запрашиваем все айтемы. Опять же нарушая все зоны ответсвенности

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants